Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New attached windows need to be placed #3676

Merged
merged 4 commits into from
Nov 26, 2024
Merged

New attached windows need to be placed #3676

merged 4 commits into from
Nov 26, 2024

Conversation

AlanGriffiths
Copy link
Collaborator

Alternative to #3675

@AlanGriffiths AlanGriffiths requested a review from a team as a code owner November 19, 2024 10:02
@AlanGriffiths AlanGriffiths mentioned this pull request Nov 19, 2024
Closed
mattkae
mattkae previously requested changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not fix the problem that I was having with cosmic-bg because LayerSurfaceV1::configure() happens before BasicWindowManager::place_new_surface(). The surface has not yet been placed when we send the configure event

@AlanGriffiths
Copy link
Collaborator Author

That sequence seems ... wrong. But I see it too:

[ 635173.015]  -> wl_compositor#6.create_surface(new id wl_surface#11)
[ 635173.030]  -> zwlr_layer_shell_v1#8.get_layer_surface(new id zwlr_layer_surface_v1#12, wl_surface#11, wl_output#3, 0, "wallpaper")
[ 635173.045]  -> zwlr_layer_surface_v1#12.set_anchor(15)
[ 635173.052]  -> zwlr_layer_surface_v1#12.set_exclusive_zone(-1)
[ 635173.058]  -> zwlr_layer_surface_v1#12.set_keyboard_interactivity(0)
[ 635173.066]  -> wl_surface#11.commit()
[ 635173.073]  -> wp_viewporter#9.get_viewport(new id wp_viewport#13, wl_surface#11)
[ 635173.102]  -> wp_fractional_scale_manager_v1#10.get_fractional_scale(new id wp_fractional_scale_v1#14, wl_surface#11)
[ 635173.788] zwlr_layer_surface_v1#12.configure(9, 0, 0)
[ 635173.807] zwlr_layer_surface_v1#12.configure(10, 1600, 900)

@AlanGriffiths
Copy link
Collaborator Author

The surface has not yet been placed when we send the configure event

Fixed now

@mattkae
Copy link
Contributor

mattkae commented Nov 21, 2024

This makes it not crash 👍 But it doesn't make it work for cosmic-bg unlike my other PR. I'll investigate and let you know why. They appear the same on the surface

@AlanGriffiths AlanGriffiths mentioned this pull request Nov 22, 2024
Open
@mattkae
Copy link
Contributor

mattkae commented Nov 22, 2024

I have fixed it with the latest commit @AlanGriffiths:

image

@Saviq
Copy link
Collaborator

Saviq commented Nov 22, 2024

32 - wlcs.Anchor/LayerSurfaceLayoutTest.* (Failed)

:)

@AlanGriffiths
Copy link
Collaborator Author

32 - wlcs.Anchor/LayerSurfaceLayoutTest.* (Failed)

:)

I understand why that broke, working on a fix

@AlanGriffiths
Copy link
Collaborator Author

Oops! Pulled the rug out of another corner

@AlanGriffiths AlanGriffiths force-pushed the place-new-attached-windows branch from bcd98f3 to bb8b9d4 Compare November 25, 2024 16:58
@AlanGriffiths
Copy link
Collaborator Author

@mattkae I'm not sure what the essential behaviour you want from 9247cc9 is. I've fixed the stuff it broke, but now not sure if the last two commits are even changing anything. (A shame you couldn't add a test)

@AlanGriffiths AlanGriffiths dismissed mattkae’s stale review November 26, 2024 09:02

Mattermost comment that "That [latest commit] did not break me"

@AlanGriffiths
Copy link
Collaborator Author

Not sure why (but I never tracked down the cause) this also seems to fix some intermittent yambar crashing

mattkae
mattkae approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working well for me now 👍

@mattkae mattkae added this pull request to the merge queue Nov 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 26, 2024
@mattkae mattkae added this pull request to the merge queue Nov 26, 2024
Merged via the queue into main with commit 9e1ccc8 Nov 26, 2024
36 of 39 checks passed
@mattkae mattkae deleted the place-new-attached-windows branch November 26, 2024 15:48
@AlanGriffiths AlanGriffiths mentioned this pull request Nov 28, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Dec 3, 2024
@Saviq
Release 2.19.0 (#3686)
699dcde 
# rc1

  * New upstream release 2.19.0

    - ABI summary:
      . miral ABI unchanged at 7
      . mircommon ABI unchanged at 10
      . mircore ABI unchanged at 2
      . miroil ABI unchanged at 5
      . mirplatform ABI bumped to 30
      . mirserver ABI bumped to 61
      . mirwayland ABI unchanged at 5
      . mirplatformgraphics ABI unchanged at 22
      . mirinputplatform ABI unchanged at 9

    - Enhancements:
. [x11-kiosk] defer fullscreening of applications until they are placed
once (#3670)
. [x11-kiosk] Ensure windows are placed and repainted on X11 "CONFIGURE"
(#3619)
      . [x11-kiosk] change enable-x11 default to true
      . Initial atomic-kms platform
      . [Wayland] Add support for xdg_activation_v1 (#3639)
      . Do not default window size (especially to weird values) (#3623)
. DesktopFileManager::resolve_app_id no longer returns an app id with a
.desktop file suffix (Fixes #3608)
. [Configuration] Split options into global and per-module configuration
(#3590)

    - Bugs fixed:
      . Ensure we always send an initial output enter (#3680)
      . Fix stuck-frame-after-mode-switch bug (#3673)
. Fix rendering of resized XWayland applications with client side
decorations (#3587)
      . Aspect ratios shouldn't contain zero (Fixes: #3663)
      . Use PkgConfig to find development headers (#3661)
. Fix the inotify event handling in
ReloadingYamlFileDisplayConfig::auto_reload() (#3636)
. The `miral::ConfigFile` "Watcher" can be destroyed before the main
loop (Fixes: #3612)
      . [xwayland] Don't allow clients to place X11 windows (#3622)
. Surfaces track scale changes on outputs they appear on. (Fixes: #3552)
      . New attached windows need to be placed (#3676)

----
[Test
Plan](https://canonical-mir.readthedocs-hosted.com/latest/how-to/how-to-test-mir-for-a-release/)

### Platforms
|| 24.04 | 24.10 |
|-|-|-|
| gbm-kms |@AlanGriffiths|@AlanGriffiths|
| atomic-kms |@AlanGriffiths|@AlanGriffiths|
| eglstream-kms |@tarek-y-ismail||
| eglstream-kms + gbm-kms hybrid |@tarek-y-ismail||
| x11 |@AlanGriffiths|@AlanGriffiths|
| wayland |@AlanGriffiths|@AlanGriffiths|
| virtual |@AlanGriffiths|@AlanGriffiths|

### Console Providers
|| 24.04 | 24.10 |
|-|-|-|
| vt |@AlanGriffiths|@AlanGriffiths|
| logind |@AlanGriffiths|@AlanGriffiths|
| minimal |@AlanGriffiths||

### Window Manager Examples
|| 24.04 | 24.10 |
|-|-|-|
| --window-manager=floating |@AlanGriffiths|@AlanGriffiths|
| --window-manager=tiling |@AlanGriffiths||
| -kiosk |@AlanGriffiths||
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattkae mattkae mattkae approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlanGriffiths @mattkae @Saviq